refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests #60

Copilot · 2026-01-10T07:05:10Z

Pull Request

Description

Replaced overly broad module-level #![allow(unused_assignments)] with targeted field-level attributes in SASL authentication structs. Also removed incorrect #[zeroize(skip)] attributes that were preventing proper memory zeroization of sensitive credential data. Added comprehensive test coverage to verify security-critical zeroization behavior.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Performance improvement
Code refactoring

Changes Made

Removed module-level #![allow(unused_assignments)] from crates/rustirc-core/src/auth.rs
Added field-level #[allow(unused_assignments)] to fields in SaslCredentials (username, password, authzid) and SecureString (inner) that derive Zeroize/ZeroizeOnDrop
Removed #[zeroize(skip)] from SaslCredentials.password and SecureString.inner to enable proper memory clearing
Added 4 new tests to verify zeroization behavior:
- test_secure_string_zeroization: Verifies SecureString properly zeroes memory
- test_sasl_credentials_zeroization: Verifies SaslCredentials properly zeroes all fields
- test_secure_string_zeroize_on_drop: Compile-time verification of ZeroizeOnDrop trait implementation
- test_sasl_credentials_zeroize_on_drop: Compile-time verification of ZeroizeOnDrop trait implementation

Before:

#![allow(unused_assignments)] // Suppresses entire module

#[derive(Debug, Clone, Zeroize, ZeroizeOnDrop)]
pub struct SaslCredentials {
    pub username: String,
    #[zeroize(skip)]  // Prevents zeroization
    pub password: SecureString,
    pub authzid: Option<String>,
}

After:

#[derive(Debug, Clone, Zeroize, ZeroizeOnDrop)]
pub struct SaslCredentials {
    #[allow(unused_assignments)]  // Targeted suppression
    pub username: String,
    #[allow(unused_assignments)]  // Targeted suppression
    pub password: SecureString,   // Now properly zeroized
    #[allow(unused_assignments)]  // Targeted suppression
    pub authzid: Option<String>,
}

Testing

My code follows the project's style guidelines
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Test Details

Clippy with -D warnings: All checks pass, no warnings generated
Unit tests: All 14 unit tests pass (4 new tests added)
Doc tests: All 24 doc tests in rustirc-core pass
Zeroization tests use ManuallyDrop to safely verify memory is zeroed before deallocation

Screenshots (if applicable)

N/A

Performance Impact

This change has no performance impact
This change improves performance
This change may reduce performance (explain why it's worth it)

Security Considerations

This change has no security implications
This change improves security
This change requires security review (explain)

Removing #[zeroize(skip)] ensures sensitive credential fields are properly cleared from memory on drop. New tests verify this security-critical behavior works correctly.

Breaking Changes

This PR introduces no breaking changes
This PR introduces breaking changes (describe migration path)

Checklist

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Note

Focuses unused_assignments lint suppression to specific fields and enables proper zeroization of sensitive data.

Removes module-level #![allow(unused_assignments)] from crates/rustirc-core/src/auth.rs
Adds field-level #[allow(unused_assignments)] to SaslCredentials (username, password, authzid) and SecureString (inner)
Removes #[zeroize(skip)] from SaslCredentials.password and SecureString.inner, allowing Zeroize/ZeroizeOnDrop to clear memory

^{Written by Cursor Bugbot for commit 5e7be54. This will update automatically on new commits. Configure here.}

…l for targeted scope Co-authored-by: doublegate <[email protected]>

doublegate

Reviewed / Approved -- DG 1/10

Copilot

Pull request overview

This PR refactors lint suppression in the SASL authentication module to replace broad module-level #![allow(unused_assignments)] with targeted field-level attributes. Additionally, it removes incorrect #[zeroize(skip)] attributes that were preventing proper memory zeroization of sensitive credential data.

Changes:

Removed module-level #![allow(unused_assignments)] from auth.rs
Added field-level #[allow(unused_assignments)] to fields that trigger the lint due to the Zeroize derive macro
Removed #[zeroize(skip)] from SaslCredentials.password and SecureString.inner to enable proper security guarantees

crates/rustirc-core/src/auth.rs

doublegate · 2026-01-10T07:17:25Z

@copilot apply changes based on the comments in this thread

…critical fields Co-authored-by: doublegate <[email protected]>

doublegate

Reviewed / Approved -- DG 1/10

…grades (closes #24, #46-56) (#59) * chore(deps): Consolidate dependency updates and GitHub Actions upgrades This PR consolidates updates from multiple open dependency PRs: ## Cargo Dependency Updates Applied: - criterion: 0.5.1 -> 0.8.1 (major version, benchmark framework) - ratatui: 0.29.0 -> 0.30.0 (TUI framework with breaking changes) - serde_json: 1.0.145 -> 1.0.148 - rustls-pki-types: 1.0 -> 1.13.2 - tracing: 0.1.43 -> 0.1.44 - tracing-subscriber: 0.3.20 -> 0.3.22 - clap: 4.5.48 -> 4.5.53 - open: 5.0 -> 5.3.3 - regex: 1.12 -> 1.12.1 ## GitHub Actions Updates Applied: - actions/cache: v4 -> v5 - actions/upload-artifact: v5 -> v6 - actions/download-artifact: v6 -> v7 ## Breaking Changes Resolved: - ratatui 0.30: Added `clear_region` method and `Error` type to Backend trait - Fixed clippy warnings in auth.rs (Zeroize derive pattern) - Fixed clippy unnecessary_unwrap in GUI button component ## Excluded from Consolidation: - iced 0.14.0 (PR #45): Extensive breaking changes requiring major GUI refactor - Would require changes to: scrollable API, application API, Style structs, text_input::Status enum, spacing types, and more - Recommended as separate PR for dedicated migration effort ## PRs Already Merged (content in main): - PR #27, #32: Phase 4 scripting documentation already present ## Verification: - Zero compilation errors - Zero clippy warnings (with -D warnings) - 60 unit tests passing - 49 doctests passing - Release build successful Closes #24, #46, #47, #48, #49, #50, #51, #52, #53, #54, #55, #56 Related: #27, #32 (already merged) Excluded: #45 (iced 0.14.0 - breaking changes too extensive) Co-Authored-By: Claude Opus 4.5 <[email protected]> * refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests (#60) * Initial plan * refactor(auth): Move lint suppression from module-level to field-level for targeted scope Co-authored-by: doublegate <[email protected]> * test(auth): Add comprehensive zeroization test coverage for security-critical fields Co-authored-by: doublegate <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: doublegate <[email protected]> * fix(ci): Resolve all failing CI checks for PR #59 - Fix auth.rs formatting: Remove trailing whitespace and format unsafe blocks properly according to rustfmt rules - Fix dependency-review-config.yml: Remove conflicting deny-licenses (cannot have both allow-licenses and deny-licenses), use proper purl format for package specifications (pkg:cargo/package-name) - Fix Windows cargo-nextest timeout: Replace cargo install with taiki-e/install-action pre-built binaries to avoid 10+ minute compilation time that caused timeouts Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ci): Expand allowed licenses for Dependency Review check Add comprehensive license list for Rust ecosystem compatibility: - Unicode licenses: Unicode-DFS-2016, Unicode-3.0 - Compression: Zlib, zlib-acknowledgement - Mozilla: MPL-2.0 - Boost: BSL-1.0 - LLVM: Apache-2.0 WITH LLVM-exception - OpenSSL, BlueOak-1.0.0, CC-BY-3.0/4.0, WTFPL, Ring, MIT-0, NCSA Add package allowlist for crates with special license definitions: - Unicode crates (unicode-ident, unicode-normalization, etc.) - Cryptography crates (ring, webpki, rustls-webpki) - OpenSSL bindings - lab crate (low OpenSSF scorecard but essential) Remove openssl-sys from deny-packages list. Fixes Dependency Review check failure on PR #59. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ci): Remove invalid 'Ring' from allow-licenses list Ring is not a valid SPDX license identifier. The ring crate uses ISC license, which is already in the allow list. The ring package is also in the allow-dependencies-licenses list to ensure it passes checks. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ci): add unicode-properties to allow-dependencies-licenses The [email protected] crate uses "MIT/Apache-2.0" as its license string, which is not valid SPDX format (should be "MIT OR Apache-2.0"). GitHub's dependency-review-action cannot validate non-SPDX license strings. Adding the package to allow-dependencies-licenses bypasses the SPDX validation while still allowing the dependency since both MIT and Apache-2.0 are approved licenses. Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: doublegate <[email protected]>

…ity fix (#64) * chore(deps): Consolidate dependency updates and GitHub Actions upgrades This PR consolidates updates from multiple open dependency PRs: ## Cargo Dependency Updates Applied: - criterion: 0.5.1 -> 0.8.1 (major version, benchmark framework) - ratatui: 0.29.0 -> 0.30.0 (TUI framework with breaking changes) - serde_json: 1.0.145 -> 1.0.148 - rustls-pki-types: 1.0 -> 1.13.2 - tracing: 0.1.43 -> 0.1.44 - tracing-subscriber: 0.3.20 -> 0.3.22 - clap: 4.5.48 -> 4.5.53 - open: 5.0 -> 5.3.3 - regex: 1.12 -> 1.12.1 ## GitHub Actions Updates Applied: - actions/cache: v4 -> v5 - actions/upload-artifact: v5 -> v6 - actions/download-artifact: v6 -> v7 ## Breaking Changes Resolved: - ratatui 0.30: Added `clear_region` method and `Error` type to Backend trait - Fixed clippy warnings in auth.rs (Zeroize derive pattern) - Fixed clippy unnecessary_unwrap in GUI button component ## Excluded from Consolidation: - iced 0.14.0 (PR #45): Extensive breaking changes requiring major GUI refactor - Would require changes to: scrollable API, application API, Style structs, text_input::Status enum, spacing types, and more - Recommended as separate PR for dedicated migration effort ## PRs Already Merged (content in main): - PR #27, #32: Phase 4 scripting documentation already present ## Verification: - Zero compilation errors - Zero clippy warnings (with -D warnings) - 60 unit tests passing - 49 doctests passing - Release build successful Closes #24, #46, #47, #48, #49, #50, #51, #52, #53, #54, #55, #56 Related: #27, #32 (already merged) Excluded: #45 (iced 0.14.0 - breaking changes too extensive) Co-Authored-By: Claude Opus 4.5 <[email protected]> * refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests (#60) * Initial plan * refactor(auth): Move lint suppression from module-level to field-level for targeted scope Co-authored-by: doublegate <[email protected]> * test(auth): Add comprehensive zeroization test coverage for security-critical fields Co-authored-by: doublegate <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: doublegate <[email protected]> * fix(ci): Resolve all failing CI checks for PR #59 - Fix auth.rs formatting: Remove trailing whitespace and format unsafe blocks properly according to rustfmt rules - Fix dependency-review-config.yml: Remove conflicting deny-licenses (cannot have both allow-licenses and deny-licenses), use proper purl format for package specifications (pkg:cargo/package-name) - Fix Windows cargo-nextest timeout: Replace cargo install with taiki-e/install-action pre-built binaries to avoid 10+ minute compilation time that caused timeouts Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ci): Expand allowed licenses for Dependency Review check Add comprehensive license list for Rust ecosystem compatibility: - Unicode licenses: Unicode-DFS-2016, Unicode-3.0 - Compression: Zlib, zlib-acknowledgement - Mozilla: MPL-2.0 - Boost: BSL-1.0 - LLVM: Apache-2.0 WITH LLVM-exception - OpenSSL, BlueOak-1.0.0, CC-BY-3.0/4.0, WTFPL, Ring, MIT-0, NCSA Add package allowlist for crates with special license definitions: - Unicode crates (unicode-ident, unicode-normalization, etc.) - Cryptography crates (ring, webpki, rustls-webpki) - OpenSSL bindings - lab crate (low OpenSSF scorecard but essential) Remove openssl-sys from deny-packages list. Fixes Dependency Review check failure on PR #59. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ci): Remove invalid 'Ring' from allow-licenses list Ring is not a valid SPDX license identifier. The ring crate uses ISC license, which is already in the allow list. The ring package is also in the allow-dependencies-licenses list to ensure it passes checks. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ci): add unicode-properties to allow-dependencies-licenses The [email protected] crate uses "MIT/Apache-2.0" as its license string, which is not valid SPDX format (should be "MIT OR Apache-2.0"). GitHub's dependency-review-action cannot validate non-SPDX license strings. Adding the package to allow-dependencies-licenses bypasses the SPDX validation while still allowing the dependency since both MIT and Apache-2.0 are approved licenses. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(security): Patch RUSTSEC-2026-0002 lru soundness vulnerability Apply security fix for vulnerable lru 0.12.5 in iced_glyphon dependency. Security Fix Applied: - Vendor patched iced_glyphon 0.6.0 with lru updated to 0.16.3 - Add Cargo patch to use vendored version - Resolves RUSTSEC-2026-0002 (IterMut violating Stacked Borrows) Dependency Chain Fixed: rustirc -> rustirc-gui -> iced 0.13.1 -> iced_wgpu -> iced_glyphon -> lru Code Quality Improvements: - Add Default derive to PluginCapabilities (clippy::derivable_impls) - Add dead_code allows for reserved Phase 4+ fields in ScriptApi Related to PR #45 (iced 0.14.0). Full iced migration deferred as it requires 82+ breaking API changes - recommended for separate PR. PRs #27, #32 superseded - Phase 4 documentation already in main branch. Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: doublegate <[email protected]>

Initial plan

7065978

Copilot AI assigned Copilot and doublegate Jan 10, 2026

Copilot AI mentioned this pull request Jan 10, 2026

chore(deps): Consolidate all dependency updates and GitHub Actions upgrades (closes #24, #46-56) #59

Merged

9 tasks

Copilot started work on behalf of doublegate January 10, 2026 07:05 View session

refactor(auth): Move lint suppression from module-level to field-leve…

5e7be54

…l for targeted scope Co-authored-by: doublegate <[email protected]>

Copilot AI changed the title ~~[WIP] Address feedback on dependency consolidation and GitHub Actions upgrades~~ refactor(auth): Replace module-level lint suppression with field-level attributes Jan 10, 2026

Copilot AI requested a review from doublegate January 10, 2026 07:13

Copilot finished work on behalf of doublegate January 10, 2026 07:13

doublegate approved these changes Jan 10, 2026

View reviewed changes

doublegate marked this pull request as ready for review January 10, 2026 07:14

Copilot AI review requested due to automatic review settings January 10, 2026 07:14

Copilot started reviewing on behalf of doublegate January 10, 2026 07:15 View session

Copilot AI reviewed Jan 10, 2026

View reviewed changes

crates/rustirc-core/src/auth.rs Show resolved Hide resolved

crates/rustirc-core/src/auth.rs Show resolved Hide resolved

Copilot started work on behalf of doublegate January 10, 2026 07:17 View session

test(auth): Add comprehensive zeroization test coverage for security-…

b881ba7

…critical fields Co-authored-by: doublegate <[email protected]>

Copilot AI changed the title ~~refactor(auth): Replace module-level lint suppression with field-level attributes~~ refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests Jan 10, 2026

Copilot AI requested a review from doublegate January 10, 2026 07:23

Copilot finished work on behalf of doublegate January 10, 2026 07:23

doublegate approved these changes Jan 10, 2026

View reviewed changes

doublegate merged commit ed748d4 into feature/consolidate-all-prs Jan 10, 2026

doublegate deleted the copilot/sub-pr-59 branch January 10, 2026 07:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests #60

refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests #60

Uh oh!

Copilot AI commented Jan 10, 2026 •

edited

Loading

Uh oh!

doublegate left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

doublegate commented Jan 10, 2026

Uh oh!

doublegate left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests #60

refactor(auth): Replace module-level lint suppression with field-level attributes and add zeroization tests #60

Uh oh!

Conversation

Copilot AI commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Description

Type of Change

Changes Made

Testing

Test Details

Screenshots (if applicable)

Performance Impact

Security Considerations

Breaking Changes

Checklist

Uh oh!

doublegate left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

doublegate commented Jan 10, 2026

Uh oh!

doublegate left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Jan 10, 2026 •

edited

Loading